Skip to content

Add animal-sniffer to guard against Java 8 only APIs#264

Merged
ZacSweers merged 3 commits intouber:masterfrom
ShaishavGandhi:sg/animal-sniffer
Oct 14, 2018
Merged

Add animal-sniffer to guard against Java 8 only APIs#264
ZacSweers merged 3 commits intouber:masterfrom
ShaishavGandhi:sg/animal-sniffer

Conversation

@ShaishavGandhi
Copy link
Copy Markdown
Collaborator

@ShaishavGandhi ShaishavGandhi commented Oct 11, 2018

Description: Add animal-sniffer to guard against Java 8 only APIs

Resolves #261

Signed-off-by: shaishavgandhi05 <shaishgandhi@gmail.com>
@ZacSweers
Copy link
Copy Markdown
Collaborator

We do to need to configure which java version?

@ShaishavGandhi
Copy link
Copy Markdown
Collaborator Author

It's here

I've added it as 1.7 since my understanding was that animal-sniffer will check if it compiles to Java 1.7. Since we don't want any Java8-only APIs, 1.7 would work.

@ZacSweers
Copy link
Copy Markdown
Collaborator

Ah gotcha, missed that on the first read. Could we try a sample commit in this branch to confirm it fails the CI checks? Then we can revert the commit after and merge if it works as expected 👍

Signed-off-by: shaishavgandhi05 <shaishgandhi@gmail.com>
@ShaishavGandhi
Copy link
Copy Markdown
Collaborator Author

Pushed a commit that fails the check. Let me know if it looks good and I can revert it.

@ZacSweers
Copy link
Copy Markdown
Collaborator

Perfect! Looks good, let’s revert and we can merge

@ShaishavGandhi
Copy link
Copy Markdown
Collaborator Author

Done!

Copy link
Copy Markdown
Collaborator

@ZacSweers ZacSweers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ZacSweers ZacSweers merged commit 327da40 into uber:master Oct 14, 2018
@ShaishavGandhi ShaishavGandhi deleted the sg/animal-sniffer branch March 18, 2019 18:08
@ShaishavGandhi ShaishavGandhi restored the sg/animal-sniffer branch March 18, 2019 18:09
ShaishavGandhi added a commit that referenced this pull request Mar 18, 2019
I was looking at integrating Animal Sniffer elsewhere and I found that the integration in AutoDispose was non-existent. In #264, I added a Java 8 API to show that Animal Sniffer works. Then when I was supposed to revert the Java 8 API commit, I reverted actual animal sniffer integration. 

![source](https://user-images.githubusercontent.com/2227639/54553592-c792e080-496f-11e9-8d71-a8cbb06e79eb.gif)

So now, this build will fail because the Java 8 API is still existent (it didn't harm anyone since it was only a test that wasn't used by anyone). I'll remove it once this build fails on CI.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants